Fix-ImproveCardDrawingLogic #42
Conversation
backend/uno-game-engine/engine.ts
Outdated
| } | ||
| } | ||
|
|
||
| shuffleDeck(deck) { |
There was a problem hiding this comment.
This is already defined in deck.ts. We'd reuse that here.
Abhishek-Punhani
force-pushed
the
FixIssue#8
branch
2 times, most recently
from
7ebc181 to
771b615
Compare
backend/uno-game-engine/engine.ts
Outdated
| } | ||
| // Move thrown cards back to the deck | ||
| this.cardDeck = this.thrownCards; | ||
| this.thrownCards = []; |
There was a problem hiding this comment.
We should have at least one thrown card since for progressing the game.
There was a problem hiding this comment.
but it checks throwncard lenght before throwing
if (this.cardDeck.length === 0) {
if (this.thrownCards.length === 0) {
throw new Error('No cards left to draw');
}
// Move thrown cards back to the deck
this.cardDeck = this.thrownCards;
this.thrownCards = [];
There was a problem hiding this comment.
I mean we shouldn't put all the thrown cards back to the deck. We reserve one card there, so that the game can progress (since the next move depends on the last card thrown)
There was a problem hiding this comment.
ohh srry lemme update and add last thrown card also
There was a problem hiding this comment.
You should be a bit more formal.(●'◡'●)
Abhishek-Punhani
force-pushed
the
FixIssue#8
branch
2 times, most recently
from
9411f44 to
42825e3
Compare
backend/uno-game-engine/engine.ts
Outdated
|
|
||
| // Move thrown cards back to the deck and updated last thrown card | ||
| this.cardDeck = this.thrownCards.slice(0, -1); | ||
| this.lastThrownCard = this.thrownCards[this.thrownCards.length - 1]; |
There was a problem hiding this comment.
On second thought, lastThrownCard is redundant as we can always just take the last element of the thrownCards array.
I think it would be nice to convert lastThrownCard into a function, which just returns the last element of thrownCards, or null if it is empty.
And here, you can use Array.splice to remove all but the last element of thrownCards and transfer it to cardDeck.
There was a problem hiding this comment.
Also, break the excessively long line in the commit message into multiple lines.
There was a problem hiding this comment.
Yes I added updateLastThrownCard function , kindly review the changes
backend/uno-game-engine/engine.ts
Outdated
| @@ -1,6 +1,7 @@ | |||
| import { getShuffledCardDeck } from './deck'; | |||
| import { handleEvent } from './gameEvents'; | |||
|
|
|||
| import { shuffle } from './deck'; | |||
| import { isUndefined } from 'util'; | |||
There was a problem hiding this comment.
I forgot to remove this , earlier i was thinking to use this function but then i removed it
Fixed the card drawing logic which would also manage moving thrown cards to deck Before drawing the card we check that that deck has sufficient card , if not we move thrown cards back to the deck otherwise we draw a card and push into the cards of the current player Fixes:shivansh-bhatnagar18#8
Abhishek-Punhani
force-pushed
the
FixIssue#8
branch
from
42825e3 to
210a58e
Compare
|
Great work here! |
backend/engine.js
Fixed the card drawing logic which would draw cards from deck and move thrown cards back to deck once the deck is empty
Fixes: #8